Skip to content

Skip a SourceFileSet that belongs to another language#77

Merged
alexander-yevsyukov merged 9 commits into
masterfrom
claude/reverent-austin-7fdd8e
Jul 1, 2026
Merged

Skip a SourceFileSet that belongs to another language#77
alexander-yevsyukov merged 9 commits into
masterfrom
claude/reverent-austin-7fdd8e

Conversation

@alexander-yevsyukov

@alexander-yevsyukov alexander-yevsyukov commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Fixes #17.

Problem

Renderer.renderSources() filtered the given SourceFileSet to the files written in the renderer's language, but still invoked the abstract render() when that filtered subset was empty because the set belonged to another language.

Source and target roots are language-specific directories (.../main/java, .../main/kotlin, …), and every renderer is invoked on every source set. So with Kotlin enabled, a Java renderer received the Kotlin-only set (rooted under .../main/kotlin) and crashed while looking up a .java file there:

java.lang.IllegalArgumentException: File not found: `io/spine/tools/compiler/test/ProjectId.java`.
  Input root:  .../consumer/build/generated/sources/proto/main/kotlin
  at io.spine.tools.compiler.render.SourceFileSet.file(SourceFileSet.kt)
  at io.spine.tools.compiler.test.uuid.UuidJavaRenderer.render(UuidJavaRenderer.java)
  at io.spine.tools.compiler.render.Renderer.renderSources(Renderer.kt)

Fix

Skip rendering when the set has files but none match the renderer's language:

val belongsToAnotherLanguage = relevantFiles.isEmpty && !sources.isEmpty
if (belongsToAnotherLanguage) return

The !sources.isEmpty half is the key distinction from a naive relevantFiles.isEmpty() guard (which broke PipelineSpec): a truly empty set is still passed to render(), so renderers that create files from scratch (e.g. InternalAccessRenderer) keep working.

UuidJavaRenderer keeps its per-file findFile(...) guard: select(UuidType.class) returns UUID types from every source root, so when a Java-producing root other than java is present (e.g. grpc), the renderer must still skip types whose .java file is absent from the current set. (An earlier revision of this PR removed that guard; review correctly flagged it is still needed, so it is retained — only its comment is clarified.)

Changes

  • Renderer.renderSources() — the language guard + doc.
  • RendererSpec — 3 regression tests: skip on another language, render on matching language, render on an empty set.
  • UuidJavaRenderer — retains the per-file findFile(...) guard for multi-root safety (net change vs master is a clearer comment).
  • version.gradle.kts2.0.0-SNAPSHOT.057.058 (version gate), as its own commit.

Verification

  • RendererSpec 6/6, PipelineSpec 17/17, plus cli / jvm / api-tests.
  • Clean integrationTest --no-build-cache: BUILD SUCCESSFUL — the Kotlin-enabled consumer / in-place-consumer now resolve the fixed compiler-cli-all:…058 and generate code without the crash and without the workaround.

Notes for the reviewer

  • The version bump is a separate commit (Bump version -> `2.0.0-SNAPSHOT.058`), and the regenerated docs/dependencies/ reports are committed on their own (Update dependency reports).
  • The third commit adds a team-memory note under .agents/memory/ about a build gotcha encountered here: after a version bump, an incremental integrationTest can launch the previous compiler because the plugin's embedded version resource isn't regenerated — verify with a clean build. It is intentionally a standalone commit so it can be reviewed (or dropped) independently of the fix.

🤖 Generated with Claude Code

alexander-yevsyukov and others added 4 commits June 30, 2026 19:48
`Renderer.renderSources()` filtered the given source set to the files
written in the renderer's language, but still called `render()` when the
filtered subset was empty because the set belonged to another language.

Source and target roots are language-specific directories (e.g.
`.../main/java`, `.../main/kotlin`), and every renderer is invoked on
every set. So, with Kotlin enabled, a Java renderer received the
Kotlin-only set rooted under `.../main/kotlin` and crashed while trying
to look up a `.java` file there.

Skip rendering when the set has files but none are in the renderer's
language. An empty set is still passed through, so renderers that create
files from scratch keep working.

Remove the now-unnecessary per-file workaround in `UuidJavaRenderer`.

Fixes #17.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Document that an incremental `integrationTest` can launch the previous
compiler version after a version bump, because the Gradle plugin's
embedded version resource is not regenerated. Verify integration with a
clean build.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Regenerated by the build after bumping the version to 2.0.0-SNAPSHOT.058.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 30, 2026 19:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a crash that occurred when a renderer was invoked with a SourceFileSet rooted in another language’s directory (e.g., Java renderer receiving a Kotlin-only set), by skipping rendering for non-empty sets that contain no files matching the renderer’s language, while still allowing truly empty sets to be rendered for “create-from-scratch” renderers.

Changes:

  • Updated Renderer.renderSources() to skip non-empty source sets that contain no files in the renderer’s language (but still render truly empty sets).
  • Added regression tests in RendererSpec covering: matching-language rendering, other-language skipping, and empty-set rendering.
  • Removed the temporary per-file workaround from UuidJavaRenderer now that Renderer.renderSources() properly guards the call.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
api/src/main/kotlin/io/spine/tools/compiler/render/Renderer.kt Adds a language-aware guard to skip rendering for non-empty sets that belong to another language; updates KDoc accordingly.
api/src/test/kotlin/io/spine/tools/compiler/render/RendererSpec.kt Adds regression tests for rendering/skip behavior across matching language, other language, and empty sets.
tests/compiler-extension/src/main/java/io/spine/tools/compiler/test/uuid/UuidJavaRenderer.java Removes the workaround that previously avoided failing on mismatched-language source roots.
version.gradle.kts Bumps snapshot version from .057 to .058 for publishing / version gating.
docs/dependencies/pom.xml Updates the dependency-report POM version to match the bumped snapshot.
docs/dependencies/dependencies.md Regenerates dependency report content for the bumped snapshot version.
.agents/memory/project/integration-tests-stale-compiler-after-version-bump.md Skipped (config-managed .agents/** file per repo review rules).
.agents/memory/MEMORY.md Skipped (config-managed .agents/** file per repo review rules).

Comment thread api/src/main/kotlin/io/spine/tools/compiler/render/Renderer.kt

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 263139da85

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Restore the `findFile` guard in `UuidJavaRenderer` (net change vs master is an
improved comment). `select(UuidType.class)` returns UUID types from every
source root, but a `SourceFileSet` covers a single root; when a Java-producing
root other than `java` exists (e.g. `grpc`), `Renderer.renderSources()` still
invokes this renderer because the set has `.java` files, so it must skip the
types whose file is absent from the current set. Flagged in PR review.

The framework guard in `Renderer.renderSources()` still fixes issue #17 (a
Kotlin-only set is never passed to a Java renderer) and is covered by
`RendererSpec`; the per-file guard handles the orthogonal multi-Java-root case.

Review nits:
- `Renderer.renderSources` KDoc: singular `sources contains`.
- `RendererSpec`: name the `input`/`output` arguments at the `sourceSetWith`
  call sites to avoid silent swaps of the same-typed `Path` parameters.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@alexander-yevsyukov alexander-yevsyukov self-assigned this Jun 30, 2026
@alexander-yevsyukov alexander-yevsyukov moved this to 🏗 In progress in v2.0 Jun 30, 2026
alexander-yevsyukov and others added 3 commits June 30, 2026 20:52
In-place re-runs of "Build on Ubuntu" kept hitting the known feature-branch build-cache flake (green on master, reproduces on unrelated docs-only branches). This empty commit forces fresh CI runs on new runners.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 30, 2026 21:16
@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.62%. Comparing base (040ec9e) to head (ab70f14).

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #77      +/-   ##
============================================
+ Coverage     75.60%   75.62%   +0.01%     
  Complexity      677      677              
============================================
  Files           203      203              
  Lines          3947     3950       +3     
  Branches        390      392       +2     
============================================
+ Hits           2984     2987       +3     
  Misses          845      845              
  Partials        118      118              
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@alexander-yevsyukov alexander-yevsyukov moved this from 🏗 In progress to In Review in v2.0 Jun 30, 2026
@alexander-yevsyukov alexander-yevsyukov merged commit e7028ab into master Jul 1, 2026
11 checks passed
@alexander-yevsyukov alexander-yevsyukov deleted the claude/reverent-austin-7fdd8e branch July 1, 2026 08:21
@github-project-automation github-project-automation Bot moved this from In Review to ✅ Done in v2.0 Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Empty Kotlin SourceFileSet is passed to JavaPrinter

3 participants